Add --quiet flag to x.py and bootstrap to suppress output#154616
Add --quiet flag to x.py and bootstrap to suppress output#154616erickt wants to merge 1 commit intorust-lang:mainfrom
--quiet flag to x.py and bootstrap to suppress output#154616Conversation
This adds a `--quiet` flag to x.py and bootstrap to suppress some of the output when compiling Rust. It works by passing the flag down to the underlying cargo, cmake, ninja, or make processes.
|
This PR modifies If appropriate, please update This PR changes how LLVM is built. Consider updating src/bootstrap/download-ci-llvm-stamp. |
rustbot
added
S-waiting-on-review
|
rustbot has assigned @Mark-Simulacrum. Use Why was this reviewer chosen?The reviewer was selected based on:
|
|
I can only look into these this weekend, but cc also #154587 (It would be nice to be careful that we don't use --quiet to mean different things) |
|
Ha! What a coincidence. I’m certainly intending to have the same semantics as that PR, so I think they should be compatible with each other. I’m happy to wait for it to land since it beat me by a few hours, then I can match the style with whatever you finalize on. |
|
(I think either one can land first, I was just remarking just in case we somehow have |
|
☔ The latest upstream changes (presumably #154824) made this pull request unmergeable. Please resolve the merge conflicts. |
|
Thanks for the patience. So I looked at both this PR and the #154587, and I think I would say:
I'll review this PR now. |
| "--warnings", choices=["deny", "warn", "default"], default="default" | ||
| ) | ||
| parser.add_argument("-v", "--verbose", action="count", default=0) | ||
| parser.add_argument("-q", "--quiet", action="store_const", const=-1, dest="verbose") |
There was a problem hiding this comment.
Discussion: hm, I thought about this a bit, I was wondering why this was using store_const and not store_{true,false}, and it's because this is the minimal change to repurpose verbose args/fields in bootstrap.py. Otherwise, you'd need to change all the if self.verbose: and friends to also do if self.verbose and not self.quiet etc.
I'm slightly conflicted, because now "verbose" is more like
suppress output increasing verbosity
◄───────────────── ─────────────────────────►
quiet 0 -v -vv ...
verbose := -1 0 1 2 ...
but then again, it's not really the end of the world. We may want to leave a comment here re. the intention / recycling of verbose tho.
| # verbose cargo output is very noisy, so only enable it with -vv | ||
| args.extend("--verbose" for _ in range(self.verbose - 1)) | ||
| if self.verbose < 0: | ||
| args.append("--quiet") |
There was a problem hiding this comment.
Question [MUTEX 1/2]: hm, this combination is somewhat interesting. Does cargo support --quiet --verbose?
EDIT: no, you cannot:
$ cargo check --quiet --verbose
error: cannot set both --verbose and --quietCan we also:
- Either do an exclusivity check here, or
- Reject the combination of both
--verbose/-vvet al. with--quietduring arg parsing?
I kinda don't want to have to wait until we reach cargo to know that this is not a valid combo.
| @@ -1276,6 +1278,7 @@ def parse_args(args): | |||
| "--warnings", choices=["deny", "warn", "default"], default="default" | |||
| ) | |||
| parser.add_argument("-v", "--verbose", action="count", default=0) | |||
There was a problem hiding this comment.
Suggestion [MUTEX 2/2]:
Looks like Python's argparse allows us to use "mutually-exclusive subgroups" (see https://docs.python.org/3/library/argparse.html#mutual-exclusion), I would suggest making --verbose and --quiet mutually exclusive for the reasons described in [MUTEX 1/2].
There was a problem hiding this comment.
Question: the general idea of a --quiet flag seems reasonable to me. Can you say more on your use case? I want to learn more re. what you intend to be using --quiet for. Is it just to suppress some of the "non-critical messages" for local dev?
I ask because some of the warnings can indicate real, genuine problems.
rustbot
added
S-waiting-on-author
|
Reminder, once the PR becomes ready for a review, use |
4 participants
This adds a
--quietflag to x.py and bootstrap to suppress some of the output when compiling Rust. It works by passing the flag down to the underlying cargo, cmake, ninja, or make processes.